Skip to content

Conversation

@AlinsRan
Copy link
Contributor

@AlinsRan AlinsRan commented Apr 2, 2025

Type of change:

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches
  • Documentation
  • Refactor
  • Chore
  • CI/CD or Tests

What this PR does / why we need it:

#76 Part2

kind: Consumer
metadata:
  name: consumer-sample
spec:
  gatewayRef:
    name: api7ee
  credentials:
    - type: basic-auth
      name: basic-auth-sample
      config:
        username: sample-user
        password: sample-password
  plugins:
    - name: key-auth
      config:
        key: consumer-key

todo:

  • Add secret support for consumers

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@AlinsRan AlinsRan force-pushed the feat/consumer-translator2 branch from 15a1333 to 38558c3 Compare April 2, 2025 09:46
@AlinsRan AlinsRan requested a review from dspo April 2, 2025 09:52
@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2025

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2025-04-03T03:47:45Z"
gatewayAPIChannel: standard
gatewayAPIVersion: v1.2.0
implementation:
  contact: null
  organization: API7
  project: api7-ingress-controller
  url: https://github.com/api7/api7-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    failedTests:
    - HTTPRouteCrossNamespace
    - HTTPRoutePathMatchOrder
    result: failure
    skippedTests:
    - GatewayInvalidTLSConfiguration
    - GatewaySecretInvalidReferenceGrant
    - GatewaySecretMissingReferenceGrant
    - GatewaySecretReferenceGrantAllInNamespace
    - GatewaySecretReferenceGrantSpecific
    - HTTPRouteExactPathMatching
    - HTTPRouteHTTPSListener
    - HTTPRouteHeaderMatching
    - HTTPRouteHostnameIntersection
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidCrossNamespaceParentRef
    - HTTPRouteInvalidNonExistentBackendRef
    - HTTPRouteInvalidParentRefNotMatchingSectionName
    - HTTPRouteInvalidReferenceGrant
    - HTTPRouteListenerHostnameMatching
    - HTTPRouteMatching
    - HTTPRouteMatchingAcrossRoutes
    - HTTPRoutePartiallyInvalidViaInvalidReferenceGrant
    - HTTPRouteReferenceGrant
    - HTTPRouteRequestHeaderModifier
    - HTTPRouteWeight
    statistics:
      Failed: 2
      Passed: 9
      Skipped: 22
  name: GATEWAY-HTTP
  summary: Core tests failed with 2 test failures.

spec:
gatewayRef:
name: api7ee
plugins:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dashboard 中几个认证类型的插件已经不能在 "插件" 中启用了, 这里应当与 dashboard 保持一致.
image

Copy link
Contributor Author

@AlinsRan AlinsRan Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

admin api 是可以的。我测过,ingress 模式可以启用,那个是 dashboard 拦截,后端并没有拦截,ingress 有拦截的必要吗?而且考虑还要对接 apisix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@AlinsRan AlinsRan Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的测试下个 PR 会删掉,这是不推荐的用法。

@dspo dspo requested a review from Copilot April 3, 2025 03:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces support for consumer translation, enhancing the API7 Ingress Controller by incorporating consumer-related functionality into ADC translation and associated e2e tests.

  • New e2e tests and CRDs for Consumer resources
  • Integration of consumer credentials and plugins in ADC translation
  • Updates to consumer controller indexing and reconciliation for improved status management

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/e2e/scaffold/k8s.go Added helper to apply resource changes in tests
test/e2e/e2e_test.go Updated test imports to include consumer CRDs
test/e2e/crds/consumer.go Introduced e2e tests for Consumer and Credential support
internal/provider/provider.go Extended translation context to include credentials
internal/provider/adc/translator/translator.go Added Consumers field to translation result
internal/provider/adc/translator/consumer.go Implemented Consumer translation for ADC
internal/provider/adc/adc.go Integrated consumer support in ADC update and delete workflows
internal/controller/status.go Introduced status condition helpers for consumer resources
internal/controller/indexer/indexer.go Added indexer for consumer gateway reference
internal/controller/consumer_controller.go Updated consumer controller with indexing, reconciliation, and secret processing
config/crd/bases/gateway.apisix.io_pluginconfigs.yaml Minor CRD adjustments
config/crd/bases/gateway.apisix.io_consumers.yaml Updated CRD defaults and validation for Consumer resources
api/adc/types.go Updated consumer type definitions and name composition

@AlinsRan AlinsRan requested a review from Copilot April 3, 2025 03:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements support for translating Consumer resources in the ADC provider and adds corresponding end-to-end tests. Key changes include:

  • Adding new e2e test cases and scaffold functions for Consumer resources.
  • Implementing consumer translation logic including credentials handling in the ADC translator.
  • Updating CRD definitions, indexers, and controllers to support Consumer resource reconciliation.

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/e2e/scaffold/k8s.go Added ResourceApplied helper for resource creation and status check.
test/e2e/e2e_test.go Updated imports to include new CRDs for consumers.
test/e2e/crds/consumer.go Added comprehensive e2e tests for Consumer plugins and credentials.
internal/provider/provider.go Extended TranslateContext with Credentials field.
internal/provider/adc/translator/translator.go Introduced Consumers field in the translation result.
internal/provider/adc/translator/httproute.go Added nil-check before unmarshalling plugin config data.
internal/provider/adc/translator/consumer.go Implemented translation logic for Consumer resources.
internal/provider/adc/adc.go Updated ADC update and delete flow to include Consumer resources.
internal/controller/status.go Introduced status helper functions for consumer status conditions.
internal/controller/indexer/indexer.go Added Consumer indexer using a new ConsumerGatewayRef field.
internal/controller/consumer_controller.go Updated reconciliation for Consumer resources including secret fetching and status update.
config/crd/bases/gateway.apisix.io_consumers.yaml Updated CRD definitions with defaults and validation for gatewayRef.
api/v1alpha1/consumer_types.go Made gatewayRef.Name required and added default values for Kind and Group.
api/adc/types.go Updated Consumer type and added ComposeConsumerName helper function.
Comments suppressed due to low confidence (1)

test/e2e/crds/consumer.go:223

  • The variable name 'updateCrendential' appears to be misspelled. Consider renaming it to 'updateCredential' for clarity.
var updateCrendential = `apiVersion: gateway.apisix.io/v1alpha1

@AlinsRan AlinsRan merged commit aebd2af into release-v2-dev Apr 3, 2025
8 checks passed
@AlinsRan AlinsRan deleted the feat/consumer-translator2 branch April 3, 2025 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants